Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Stricter JSON parse when content-type header is application/json. #2

Merged
merged 1 commit into from
Dec 5, 2012
Merged

Stricter JSON parse when content-type header is application/json. #2

merged 1 commit into from
Dec 5, 2012

Conversation

willwhite
Copy link
Collaborator

Just a little fix to address a problem we had during the Chargify datacenter migration last weekend. Their API was not returning a complete response and therefore the JSON was invalid. Instead of being caught at the source, the unpatched code would allow the body to slip by as a string, causing potential havoc downstream when code get a string instead of an object.

Would love to get this in a release if you have a moment, @natevw.

Thanks!

natevw added a commit that referenced this pull request Dec 5, 2012
Stricter JSON parse when content-type header is application/json.
@natevw natevw merged commit 61b60cc into natevw:master Dec 5, 2012
@natevw
Copy link
Owner

natevw commented Dec 5, 2012

Ah, hmm. Forgot this had switched over to mikeal's request library...so is req.body normally converted automatically and our code is just a backup? Was going to just clean up and release a minor update but please could you look into this? (I'm in CET and really must be sleeping now, but will try move this forward in my morning.)

@willwhite
Copy link
Collaborator Author

Hi @natevw. I really appreciate you looking at this so quickly. I replied to inline comments above. Happy to discuss more tomorrow and have a good night.

@natevw
Copy link
Owner

natevw commented Dec 5, 2012

Heh, didn't realize that this lib was still v0.2 so just a minor release should suffice. Just npm published version 0.3.0.

@willwhite
Copy link
Collaborator Author

Cheers. Again, grateful for the fast response.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants